Maya: write color sets in create_model #3690

moonyuet · 2022-08-18T13:30:47Z

Brief description

Adding the on/off option of write color sets attributes in create_rig.py

Adding swtiching on/off clickbox to override the boolean option of write_color_sets_arttirbute in create_model.py and create_rig.py

…/off for write_color_sets in create_model/rig

ynbot · 2022-08-18T13:30:50Z

Task linked: OP-3749 write color sets

…/off for write_color_sets in create_model/rig

BigRoy · 2022-08-18T13:52:55Z

openpype/hosts/maya/plugins/create/create_rig.py

+ write_color_sets = False
+
+ def __init__(self, *args, **kwargs):
+ super(CreateRig, self).__init__(*args, **kwargs)
+ self.data["writeColorSets"] = self.write_color_sets


This shouldn't make much sense since the rig comes out as maya scene and does not strip any color set data. Animation instances however would extract as Alembic.

we are trying to unify colors sets (and other sets) flow and even though currently rig creator can only output Maya scene we plan to add fbx representation (replacing the skeletal mesh) so it will make sense then.

It is not influencing anything currently but we thought it would be good to have it there for future use and consistency.

It is not influencing anything currently but we thought it would be good to have it there for future use and consistency

I see what you're going for - but it'd just confuse the artists into thinking it would do something useful for the rig family.

Also, most usual "rigs" won't be suitable to do an "fbx rig" export for - and then also automate animation instance creation for in maya when loading them again. I'd say fbxRig is a family of its own and should also come with its own creator, validators, etc.

If we're adding the option we should make the extractor capable of removing the values when publishing the maya scene to avoid any confusion whatsoever. Which is not an easy task for rigs with complex history.

so we have to decide if we remove the component sets from Rig Creator @antirotor

After internal discussion, we decided to keep the sets for Rig Creator there. We understand it is not very clean from the code perspective as these options are actually modifying the animation output, not the rig output itself, but as it does not have its own creator we gave preference to keeping all attributes settings in one place in preferences and not moving it outside just for the rig.
We think it will be more consistent from the user's point of view.

BigRoy · 2022-08-18T13:53:22Z

openpype/hosts/maya/plugins/publish/extract_playblast.py

+ pattern_frame = [r'\.(?P<index>(?P<padding>0*)\d+)\.\D+\d?$']
 collections, remainder = clique.assemble(collected_files,
- minimum_items=1)
+ minimum_items=1,
+ patterns=pattern_frame)


Seems unrelated to the PR description.

my bad, this is related to another ticket.

Seems unrelated to the PR description.

coming from #3683 ?

m-u-r-p-h-y · 2022-08-18T14:00:34Z

I would add Face Sets options as well to make it complete

…/off for write_color_sets in create_model/rig

moonyuet · 2022-08-18T14:14:34Z

I would add Face Sets options as well to make it complete

added into the create_rig.py

m-u-r-p-h-y · 2022-08-19T07:09:58Z

added into the create_rig.py

can you also add the Face sets to the corresponding settings?

…/off for write_color_sets in create_model/rig

m-u-r-p-h-y

the settings are correctly propagated to their respective creators.

note that the settings for Rig Creator are not modifying rig publish, but output from rig animation, which is handled automatically without its own creator.

BigRoy · 2022-08-19T13:12:58Z

note that the settings for Rig Creator are not modifying rig publish, but output from rig animation, which is handled automatically without its own creator.

Where is the code/logic for that? I don't recall seeing any Rig loader logic that sets anything like that for the to be created animation instance? Looking at the code the animation instance values have nothing to do with the setting that were set on the rig instance when being published. (That data is not even in the database or on disk, right?)

Or what obvious thing am I missing?

antirotor · 2022-09-05T15:18:17Z

Or what obvious thing am I missing?

No, but it is somewhat convoluted - animation creator is called when rig is loaded into the scene:

https://github.com/pypeclub/OpenPype/blob/377df371db52412f9485272da413448ef647107c/openpype/hosts/maya/plugins/load/load_reference.py#L155-L167

but since this creator is never called by user (and shouldn't be even listed in Creator dialog - but we can't hide it now), it's logical to move settings for it somewhere. Not saying it is ideal, but the options are:

have the settings in animation creator: that is logical from the point of code, but not very user intuitive
have that option on rig loader - but again, it actually influence subsequent publishing of animation from the loaded rig
have it on the rig creator as we have in this PR
other option? :)

BigRoy · 2022-09-06T07:08:48Z

The point I was trying to make was that the values you set on the Rig instance here in CreateRig don't seem to influence the values on the Animation instance at all. Those values are not published along with the Rig in such a way that it seems that the Rig loader set anything related on the animation in any way.

As such the values on CreateRig and the rig instance itself have no use. They do nothing, they don't propagate to the animation instance when the rig is loaded. Or where is that logic?

have the settings in animation creator: that is logical from the point of code, but not very user intuitive
have that option on rig loader - but again, it actually influence subsequent publishing of animation from the loaded rig
have it on the rig creator as we have in this PR
other option? :)

I'd say that IF the rig instance can set default values for the animation instance that could be quite useful. So that an advanced rigger could say "for this rig please publish animation with write color sets enabled by default" - I'd love to see that feature with "visibleOnly" to be settable per rig instance too. But the logic involved with trickling down decisions like that are much more complicated (the code isn't there yet as far as I know).

What happens if:

Rig instance was already loaded and an animation family was created, now the Rig version gets updated which has different values for the "write color data" published along. What do we do with the existing animation instance in the scene? Do we preserve any local adjustments a user might have made or do we force revert it?
If the Rigger allows to set a preferred value does it really make sense to allow the animator to still change the value? Like, how "strict" are these?

TL;DR

Having the settings on the animation/pointcache instance is worthwhile - but I'm not seeing how the logic added now for rig instances is doing anything useful.

mkolar · 2022-09-28T06:56:39Z

openpype/hosts/maya/plugins/create/create_rig.py

+ def __init__(self, *args, **kwargs):
+ super(CreateRig, self).__init__(*args, **kwargs)
+ self.data["writeColorSets"] = self.write_color_sets
+ self.data["writeFaceSets"] = self.write_face_sets


@BigRoy is correct. This code has absolutely no bearing on any result anywhere. When we load the rig, an animation instance is created automatically, but that uses the settings from animation creator, rather than rig. So this introduces two attributes that are just extra and will create confusion.

It would of course be possible to have these parsed into the animation instance that is created, so that rigger could choose what values should be used down the line as starting point, but I'm not sure that it's actually worth the hassle at the moment, considering that simply adding the attributes to the animation creator solves the immediate issue for the upcoming production.

that being said, the confusion between animation family, animation creator and how and if they are affected from the rig, is almighty and should be addressed. I'm not convinced it is the responsibility of this PR though, so I reckon let's just remove the obsolete code from the rig creator here and separately have a look at how to clean up the rig -> animation workflow to make it a bit more clear.

m-u-r-p-h-y · 2022-10-12T06:59:03Z

we should unblock this one as some studios are waiting for create_model at least

@BigRoy is right that the added attribute on create_rig is not doing anything so @moonyuet can you remove it pls so we can merge this pls?

or is it done already?

mkolar · 2022-10-17T13:20:41Z

openpype/settings/defaults/project_settings/maya.json

@@ -33,7 +33,7 @@
 },
 "RenderSettings": {
 "apply_render_settings": true,
- "default_render_image_folder": "",
+ "default_render_image_folder": "renders",


this seems unrelated to this PR, other than that I'm happy to merge

mkolar

All code seems good to me, let's just please remove unrelated settings change as per comment

add write_color_sets in create_rig and enable options of swtiching on…

851b573

…/off for write_color_sets in create_model/rig

moonyuet requested review from m-u-r-p-h-y and antirotor August 18, 2022 13:30

moonyuet self-assigned this Aug 18, 2022

add write_color_sets in create_rig and enable options of swtiching on…

d90fa8b

…/off for write_color_sets in create_model/rig

moonyuet added the host: Maya label Aug 18, 2022

BigRoy reviewed Aug 18, 2022

View reviewed changes

add write_color_sets in create_rig and enable options of swtiching on…

745386d

…/off for write_color_sets in create_model/rig

add write_color_sets in create_rig and enable options of swtiching on…

31cc505

…/off for write_color_sets in create_model/rig

m-u-r-p-h-y approved these changes Aug 19, 2022

View reviewed changes

m-u-r-p-h-y added the type: enhancement Enhancements to existing functionality label Aug 19, 2022

mkolar changed the title ~~Enhancement/op 3749 write color sets in create_rig and create_model~~ Maya: write color sets in create_rig and create_model Aug 24, 2022

iLLiCiTiT mentioned this pull request Sep 8, 2022

General: lock task workfiles when they are working on #3747

Closed

mkolar suggested changes Sep 28, 2022

View reviewed changes

write color sets

affd54b

moonyuet requested a review from mkolar September 28, 2022 11:11

m-u-r-p-h-y changed the title ~~Maya: write color sets in create_rig and create_model~~ Maya: write color sets in create_model Oct 12, 2022

delete unneccessary schema and data of Create Rig

c369b6e

moonyuet force-pushed the enhancement/OP-3749_write_color_sets branch from 1e710ae to c369b6e Compare October 12, 2022 07:46

mkolar reviewed Oct 17, 2022

View reviewed changes

mkolar suggested changes Oct 17, 2022

View reviewed changes

clean up

c410ee6

moonyuet requested a review from mkolar October 17, 2022 13:24

antirotor approved these changes Oct 17, 2022

View reviewed changes

mkolar approved these changes Oct 19, 2022

View reviewed changes

mkolar merged commit ed144b9 into ynput:develop Oct 19, 2022

moonyuet deleted the enhancement/OP-3749_write_color_sets branch December 13, 2022 02:15

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Maya: write color sets in create_model #3690

Maya: write color sets in create_model #3690

moonyuet commented Aug 18, 2022

ynbot commented Aug 18, 2022

BigRoy Aug 18, 2022

m-u-r-p-h-y Aug 18, 2022

BigRoy Aug 18, 2022 •

edited

Loading

m-u-r-p-h-y Aug 19, 2022

m-u-r-p-h-y Aug 19, 2022

BigRoy Aug 18, 2022

moonyuet Aug 18, 2022

m-u-r-p-h-y Aug 18, 2022

moonyuet Aug 18, 2022

m-u-r-p-h-y commented Aug 18, 2022

moonyuet commented Aug 18, 2022

m-u-r-p-h-y commented Aug 19, 2022 •

edited

Loading

m-u-r-p-h-y left a comment •

edited

Loading

BigRoy commented Aug 19, 2022

antirotor commented Sep 5, 2022

BigRoy commented Sep 6, 2022 •

edited

Loading

mkolar Sep 28, 2022 •

edited

Loading

m-u-r-p-h-y commented Oct 12, 2022 •

edited

Loading

mkolar Oct 17, 2022

mkolar left a comment

Maya: write color sets in create_model #3690

Maya: write color sets in create_model #3690

Conversation

moonyuet commented Aug 18, 2022

Brief description

ynbot commented Aug 18, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

BigRoy Aug 18, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

m-u-r-p-h-y commented Aug 18, 2022

moonyuet commented Aug 18, 2022

m-u-r-p-h-y commented Aug 19, 2022 • edited Loading

m-u-r-p-h-y left a comment • edited Loading

Choose a reason for hiding this comment

BigRoy commented Aug 19, 2022

antirotor commented Sep 5, 2022

BigRoy commented Sep 6, 2022 • edited Loading

mkolar Sep 28, 2022 • edited Loading

Choose a reason for hiding this comment

m-u-r-p-h-y commented Oct 12, 2022 • edited Loading

Choose a reason for hiding this comment

mkolar left a comment

Choose a reason for hiding this comment

BigRoy Aug 18, 2022 •

edited

Loading

m-u-r-p-h-y commented Aug 19, 2022 •

edited

Loading

m-u-r-p-h-y left a comment •

edited

Loading

BigRoy commented Sep 6, 2022 •

edited

Loading

mkolar Sep 28, 2022 •

edited

Loading

m-u-r-p-h-y commented Oct 12, 2022 •

edited

Loading